chore: add cdp connection to cy prompt #31806
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
Adds CDP support for cy.prompt by introducing a new connectCyPromptToBrowser workflow across the server, socket layers, project bootstrap, browser launchers, and tests.
- Expose and wire
connectCyPromptToBrowserinOpenProject,ProjectBase, andSocketBase - Extend each browser launcher (Chrome, Electron, Firefox, WebKit) and the core
BrowserCriClientto clone and connect a separate CDP client forcy.prompt - Update TypeScript types, fixtures, and unit tests to cover the new
CyPromptManagerandCyPromptCDPClientintegration
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/server/lib/open_project.ts | Added connectCyPromptToBrowser method |
| packages/server/lib/project-base.ts | Hooked onCyPromptReady to connect prompt manager to browser |
| packages/server/lib/socket-base.ts | Introduced onCyPromptReady socket handler |
| packages/server/lib/browsers/types.ts | Extended BrowserLauncher type with connectCyPromptToBrowser |
| packages/server/lib/browsers/index.ts | Dispatched connectCyPromptToBrowser to per-browser launcher |
| packages/server/lib/browsers/chrome.ts | Implemented connectCyPromptToBrowser for Chrome |
| packages/server/lib/browsers/electron.ts | Implemented connectCyPromptToBrowser for Electron |
| packages/server/lib/browsers/firefox.ts | Stubbed unsupported prompt with an error |
| packages/server/lib/browsers/webkit.ts | Stubbed unsupported prompt with an error |
| packages/server/lib/browsers/browser-cri-client.ts | Extended BrowserCriClient to clone and notify cyPromptManager |
| packages/server/lib/cloud/cy-prompt/CyPromptManager.ts | Added connectToBrowser API and CDP client handling |
| packages/server/test/unit/open_project_spec.js | Added a new test context for connectCyPromptToBrowser |
| packages/server/test/unit/cloud/cy-prompt/CyPromptManager_spec.ts | Updated tests for prompt manager, left TODOs for error assertions |
| packages/server/test/unit/browsers/webkit_spec.ts | Added unsupported-prompt test for WebKit |
| packages/server/test/unit/browsers/firefox_spec.ts | Added unsupported-prompt test for Firefox |
| packages/server/test/unit/browsers/electron_spec.js | Added prompt-connection tests for Electron |
| packages/server/test/unit/browsers/chrome_spec.js | Added prompt-connection tests for Chrome |
| packages/server/test/unit/browsers/browsers_spec.js | Ensured top-level dispatcher routes prompt connection |
| packages/server/test/unit/browsers/browser-cri-client_spec.ts | Added cyPromptManager to browser-cri-client tests |
| packages/server/test/support/fixtures/cloud/cy-prompt/test-cy-prompt.ts | Updated test fixture to include a no-op connectToBrowser |
Comments suppressed due to low confidence (4)
packages/server/test/unit/open_project_spec.js:275
- This test calls connectCyPromptToBrowser but does not assert that browsers.connectCyPromptToBrowser was invoked; add an expectation such as
expect(browsers.connectCyPromptToBrowser).to.be.calledWith(options).
await openProject.connectCyPromptToBrowser(options)
packages/server/test/unit/cloud/cy-prompt/CyPromptManager_spec.ts:56
- Replace this TODO with a concrete assertion verifying that an error is reported or logged when initialization fails, e.g., checking an error callback or status change.
// TODO: test that the error is reported
packages/server/lib/browsers/firefox.ts:443
- [nitpick] Inconsistent casing: consider capitalizing
Firefoxto matchWebKitand maintain a consistent style in error messages.
throw new Error('CyPrompt is not yet supported in firefox.')
packages/server/lib/cloud/cy-prompt/CyPromptManager.ts:43
- Changing
isEssentialfromtruetofalsemay suppress critical errors for cy.prompt; consider retainingtrueto ensure upstream error handling remains active.
return this.invokeAsync('handleBackendRequest', { isEssential: false }, eventName, ...args)
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
ryanm/chore/add-cdp-connection-to-cy-prompt
|
| Run status |
|
| Run duration | 19m 44s |
| Commit |
|
| Committer | Ryan Manuel |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
10
|
Pending
|
1232
|
Skipped
|
0
|
Passing
|
32187
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
45.96%
|
|
|---|---|
|
Untested elements
|
189
|
|
Tested elements
|
165
|
Accessibility
92.74%
|
|
|---|---|
|
Failed rules
|
3 critical
9 serious
2 moderate
2 minor
|
|
Failed elements
|
698
|
AtofStryker
reviewed
May 30, 2025
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
AtofStryker
approved these changes
Jun 2, 2025
| @@ -1,5 +1,10 @@ | |||
| describe('src/cy/commands/prompt', () => { | |||
| it('executes the prompt command', () => { | |||
| // TODO: test the error messages | |||
There was a problem hiding this comment.
is it possible to link this to an issue? Might be good to explain that cy.prompt() isn't supported in these browsers and that tests are coming to test the errors
There was a problem hiding this comment.
packages/server/test/unit/cloud/cy-prompt/CyPromptManager_spec.ts
Outdated
Show resolved
Hide resolved
mschile
reviewed
Jun 2, 2025
mschile
reviewed
Jun 2, 2025
mschile
reviewed
Jun 2, 2025
mschile
approved these changes
Jun 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Additional details
cy.promptwill require a CDP connection to access A11y methods. This PR adds the ability to connect the CDP connection to the cloud-deliveredcy.promptSteps to test
How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?